Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add/SparseMatricesCSR #118

Merged
merged 7 commits into from
Oct 29, 2019
Merged

Add/SparseMatricesCSR #118

merged 7 commits into from
Oct 29, 2019

Conversation

victorsndvg
Copy link
Contributor

  • Integrate SparseMatricesCSR into Gridap Assemblers and MultiAssemblers.
  • Assemblers and MultiAssemblers are able to work with AbstractSparseMatrix data type.
  • Assemblers and MultiAssemblers work with SparseMatrixCSC (from SparseArrays), SparseMatrixCSR and SymSparseMatrixCSR (from SparseMatricesCSR).
  • Tests included

@victorsndvg
Add SparseMatricesCSR
52f0a23 
SparseMatrixAssembler produces SparseMatrices also from SparseMatricesCSR packages
@victorsndvg
Finish SparseMatricesCSR integration into Assemblers.jl
f10a408 
Add SparseMAtrixCSR and SymSparseMatrixCSR to AssemblersTests.jl
@victorsndvg
MultiSparseMatrixAssembler produces SparseMatrices also from SparseMa…
0c48dd0 
…tricesCSR packages

Finish SparseMatricesCSR integration into MultiAssemblers.jl
Add SparseMAtrixCSR and SymSparseMatrixCSR to MultiAssemblersTests.jl
@victorsndvg victorsndvg added the enhancement New feature or request label Oct 28, 2019
@victorsndvg victorsndvg requested a review from fverdugo October 28, 2019 14:26
fverdugo
fverdugo reviewed Oct 28, 2019
View reviewed changes
Project.toml Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Oct 28, 2019 •
edited
Loading

Codecov Report

Merging #118 into master will decrease coverage by 0.14%.
The diff coverage is 72.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #118      +/-   ##
==========================================
- Coverage   91.59%   91.45%   -0.15%     
==========================================
  Files          90       90              
  Lines        4761     4775      +14     
==========================================
+ Hits         4361     4367       +6     
- Misses        400      408       +8
Impacted Files Coverage Δ
src/FESpaces/FEOperators.jl 78.89% <0%> (-3.01%) ⬇️
src/FESpaces/Assemblers.jl 93.1% <100%> (+0.51%) ⬆️
src/MultiField/MultiAssemblers.jl 83.95% <68.75%> (-4.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2190c7...08edf7e. Read the comment docs.

fverdugo
fverdugo reviewed Oct 28, 2019
View reviewed changes
Copy link
Member

@fverdugo fverdugo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats for the work!

A part form the comments I have done in the code, I don't see the call to finalize_coo! Have you followed another aproach?

src/FESpaces/Assemblers.jl Outdated Show resolved Hide resolved
src/FESpaces/Assemblers.jl Outdated Show resolved Hide resolved
src/FESpaces/Assemblers.jl Outdated Show resolved Hide resolved
src/FESpaces/Assemblers.jl Outdated Show resolved Hide resolved
src/FESpaces/Assemblers.jl Show resolved Hide resolved
src/MultiField/MultiAssemblers.jl Outdated Show resolved Hide resolved
src/MultiField/MultiAssemblers.jl Outdated Show resolved Hide resolved
src/MultiField/MultiAssemblers.jl Outdated Show resolved Hide resolved
src/MultiField/MultiAssemblers.jl Outdated Show resolved Hide resolved
fverdugo
fverdugo reviewed Oct 28, 2019
View reviewed changes
Copy link
Member

@fverdugo fverdugo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fverdugo
Copy link
Member

Just a detail. I still miss the finalize_coo! function. It's not needed any more?

@victorsndvg
Copy link
Contributor Author

@fverdugo , yes you are right! i forgot finalize_coo! function.

@victorsndvg
Copy link
Contributor Author

call to finalize_coo! added in 6ef127f

@fverdugo
Copy link
Member

Just a final comment @victorsndvg

The Travis CI time has doubled. Do you know if this is this related with your changes or due to another source?

@victorsndvg
Copy link
Contributor Author

Just a final comment @victorsndvg

The Travis CI time has doubled. Do you know if this is this related with your changes or due to another source?

I've to check it

@fverdugo
Copy link
Member

your branch took 23m and the PR 41m.

master has not diverged from your branch so the PR should be equal to your branch.

It seems something not related with us, right?

@fverdugo fverdugo merged commit bc9a6d1 into master Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fverdugo fverdugo fverdugo left review comments

Assignees

@fverdugo fverdugo

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@victorsndvg @codecov-io @fverdugo